Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Copy the DOM XPath interfaces from the WHATWG wiki #763

Merged
merged 5 commits into from
Aug 30, 2019
Merged

Copy the DOM XPath interfaces from the WHATWG wiki #763

merged 5 commits into from
Aug 30, 2019

Conversation

foolip
Copy link
Member

@foolip foolip commented May 29, 2019 •
edited by pr-preview bot
Loading

@foolip
Copy link
Member Author

foolip commented May 29, 2019

Tests using this IDL already exist:
https://wpt.fyi/results/domxpath/interfaces.tentative.html?run_id=258200004&run_id=264060011&run_id=250500005&run_id=266120012

They show that all of this is already implemented, the failures in Edge and Safari are because of minor differences in length properties, what exception is thrown, and so on.

@foolip foolip requested a review from annevk May 29, 2019 13:17
@foolip foolip mentioned this pull request May 29, 2019
Open
gsnedders
gsnedders reviewed May 29, 2019
View reviewed changes
dom.bs Outdated Show resolved Hide resolved
dom.bs Outdated Show resolved Hide resolved
dom.bs Outdated Show resolved Hide resolved
dom.bs Outdated Show resolved Hide resolved
dom.bs Outdated Show resolved Hide resolved
dom.bs Outdated Show resolved Hide resolved
dom.bs Outdated
[[DOM-Level-3-XPath]] [[XPath]] These APIs are widely implemented, but
<cite>DOM Level 3 XPath</cite> is no longer maintained. The DOM Standard does not attempt to
define any of the behavior, but the interface definitions are maintained here so that they can
be updated when Web IDL changes. [[WEBIDL]]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's more changes than just that, some of them simplify the API by making arguments optional.

Let's also add something, possibly just normative, saying:

  • XPathException is gone (just use DOMException)
  • The query is matched against the DOM, and therefore contrary to the XPath 1.0 data model the root element has a parent (the Document) and text nodes can be adjacent to one another. [wilful violation]

annevk
annevk reviewed Jun 28, 2019
View reviewed changes
Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know if you want to improve this further or have me do it, but there's two changes I'd like to see here before merging this:

  1. The first paragraph should be reworded and flagged as an issue (using class=issue), ideally linking to an open issue in the DOM Standard that would allow for someone to take on the work of defining the API in more detail. Sam's points can be included in that issue description. I don't think we should have normative wording outside of the issue.
  2. The note should probably be reworded a bit to be slightly more formal.

Also, if anyone has issues with merging this, please speak up, as I don't have a good reason to not do this at this point.

@foolip
Copy link
Member Author

foolip commented Jul 19, 2019

I'll polish this up next week.

@foolip
Copy link
Member Author

foolip commented Jul 25, 2019

I didn't get to this this week and I'm OOO tomorrow. Snoozing it until next week but won't be offended if someone takes it over.

@foolip
Copy link
Member Author

foolip commented Aug 28, 2019

@annevk I pushed the changes. However, class=issue doesn't seem to be a thing, so I used class=warning. Don't love the way it looks, is there a better class?

@zcorpan
Copy link
Member

zcorpan commented Aug 28, 2019

@foolip class=XXX

@foolip
Copy link
Member Author

foolip commented Aug 28, 2019

@zcorpan thanks, trying that instead

@annevk annevk mentioned this pull request Aug 28, 2019
Closed
@annevk
Copy link
Member

annevk commented Aug 28, 2019

This is blocked by w3ctag/promises-guide#27 for now. I adjusted the wording a bit to make it clear that defining these is still a goal, albeit perhaps with less priority than some would like.

@foolip
Copy link
Member Author

foolip commented Aug 30, 2019

w3ctag/promises-guide#27 is resolved, Travis now happy.

@annevk annevk merged commit dea5d92 into master Aug 30, 2019
@annevk annevk deleted the foolip/xpath-idl branch August 30, 2019 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@annevk annevk annevk left review comments

@gsnedders gsnedders gsnedders left review comments

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@foolip @zcorpan @annevk @gsnedders